🎖️GitЯра🎖️
Commit 1976808a361942f2c358329807d3605b8898e36d
Parents : 57b0200
Author : James Rich <2199651+jamesarich@users.noreply.github.com>
Signature : Signature validation error
Date : 2026-05-12T21:06:52-05:00
Committer : GitHub <noreply@github.com>
Date : 2026-05-13T02:06:52Z
refactor: use immutable destNum in RadioConfigViewModel (#5436)
Changes
3 files changed, 83 insertions(+), 52 deletions(-)
Diff
diff --git a/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsNavigation.kt b/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsNavigation.kt
index dae427f05d..0183085e12 100644
--- a/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsNavigation.kt
+++ b/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/navigation/SettingsNavigation.kt
@@ -20,12 +20,14 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember
+import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import androidx.lifecycle.compose.dropUnlessResumed
import androidx.navigation3.runtime.EntryProviderScope
import androidx.navigation3.runtime.NavBackStack
import androidx.navigation3.runtime.NavKey
import org.koin.compose.viewmodel.koinViewModel
+import org.koin.core.parameter.parametersOf
import org.meshtastic.core.navigation.NodesRoute
import org.meshtastic.core.navigation.Route
import org.meshtastic.core.navigation.SettingsRoute
@@ -71,7 +73,6 @@ import kotlin.reflect.KClass
@Composable
fun getRadioConfigViewModel(backStack: NavBackStack<NavKey>): RadioConfigViewModel {
- val viewModel = koinViewModel<RadioConfigViewModel>()
val destNum =
remember(backStack.toList()) {
backStack.lastOrNull { it is SettingsRoute.Settings }?.let { (it as SettingsRoute.Settings).destNum }
@@ -79,8 +80,9 @@ fun getRadioConfigViewModel(backStack: NavBackStack<NavKey>): RadioConfigViewMod
.lastOrNull { it is SettingsRoute.SettingsGraph }
?.let { (it as SettingsRoute.SettingsGraph).destNum }
}
- LaunchedEffect(destNum) { viewModel.initDestNum(destNum) }
- return viewModel
+ return koinViewModel<RadioConfigViewModel>(key = destNum?.toString()) {
+ parametersOf(SavedStateHandle(mapOf("destNum" to destNum)))
+ }
}
@Suppress("LongMethod", "CyclomaticComplexMethod")
diff --git a/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModel.kt b/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModel.kt
index fd923a1336..991a27d97b 100644
--- a/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModel.kt
+++ b/feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModel.kt
@@ -27,7 +27,10 @@ import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
+import kotlinx.coroutines.flow.flatMapLatest
+import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
@@ -181,13 +184,7 @@ open class RadioConfigViewModel(
_mqttProbeStatus.value = null
}
- private val destNumFlow = MutableStateFlow(savedStateHandle.get<Int>("destNum"))
-
- fun initDestNum(id: Int?) {
- if (destNumFlow.value != id) {
- destNumFlow.value = id
- }
- }
+ private val destNum: Int? = savedStateHandle.get<Int>("destNum")
private val _destNode = MutableStateFlow<Node?>(null)
val destNode: StateFlow<Node?>
@@ -209,7 +206,8 @@ open class RadioConfigViewModel(
locationService.getCurrentLocation()
init {
- combine(destNumFlow, nodeRepository.nodeDBbyNum) { id, nodes -> nodes[id] ?: nodes.values.firstOrNull() }
+ nodeRepository.nodeDBbyNum
+ .map { nodes -> if (destNum != null) nodes[destNum] else nodes.values.firstOrNull() }
.distinctUntilChanged()
.onEach {
_destNode.value = it
@@ -219,19 +217,40 @@ open class RadioConfigViewModel(
radioConfigRepository.deviceProfileFlow.onEach { _currentDeviceProfile.value = it }.launchIn(viewModelScope)
- radioConfigRepository.localConfigFlow
- .onEach { lc -> if (radioConfigState.value.isLocal) _radioConfigState.update { it.copy(radioConfig = lc) } }
- .launchIn(viewModelScope)
-
- radioConfigRepository.channelSetFlow
- .onEach { cs ->
- if (radioConfigState.value.isLocal) _radioConfigState.update { it.copy(channelList = cs.settings) }
+ // Derive isLocal from the immutable destNum and the (possibly changing) myNodeInfo.
+ // flatMapLatest cancels the previous inner flow on every change, so there is
+ // no window where stale local config can leak through.
+ nodeRepository.myNodeInfo
+ .map { ni ->
+ val isLocal = (destNum == null) || (destNum == ni?.myNodeNum)
+ isLocal
}
- .launchIn(viewModelScope)
-
- radioConfigRepository.moduleConfigFlow
- .onEach { lmc ->
- if (radioConfigState.value.isLocal) _radioConfigState.update { it.copy(moduleConfig = lmc) }
+ .distinctUntilChanged()
+ .flatMapLatest { isLocal ->
+ if (isLocal) {
+ combine(
+ radioConfigRepository.channelSetFlow,
+ radioConfigRepository.localConfigFlow,
+ radioConfigRepository.moduleConfigFlow,
+ ) { cs, lc, mc ->
+ _radioConfigState.update {
+ it.copy(isLocal = true, channelList = cs.settings, radioConfig = lc, moduleConfig = mc)
+ }
+ }
+ } else {
+ // Remote admin: clear local config once, then stay idle.
+ // Remote responses arrive via processPacketResponse.
+ flowOf(Unit).onEach {
+ _radioConfigState.update {
+ it.copy(
+ isLocal = false,
+ channelList = emptyList(),
+ radioConfig = LocalConfig(),
+ moduleConfig = LocalModuleConfig(),
+ )
+ }
+ }
+ }
}
.launchIn(viewModelScope)
@@ -250,11 +269,6 @@ open class RadioConfigViewModel(
}
.launchIn(viewModelScope)
- combine(nodeRepository.myNodeInfo, destNumFlow) { ni, id ->
- _radioConfigState.update { it.copy(isLocal = (id == null) || (id == ni?.myNodeNum)) }
- }
- .launchIn(viewModelScope)
-
Logger.d { "RadioConfigViewModel created" }
}
@@ -284,7 +298,7 @@ open class RadioConfigViewModel(
}
fun setOwner(user: User) {
- val destNum = destNode.value?.num ?: return
+ val destNum = destNum ?: destNode.value?.num ?: return
safeLaunch(tag = "setOwner") {
_radioConfigState.update { it.copy(userConfig = user) }
val packetId = radioConfigUseCase.setOwner(destNum, user)
@@ -293,7 +307,7 @@ open class RadioConfigViewModel(
}
fun updateChannels(new: List<ChannelSettings>, old: List<ChannelSettings>) {
- val destNum = destNode.value?.num ?: return
+ val destNum = destNum ?: destNode.value?.num ?: return
getChannelList(new, old).forEach { channel ->
safeLaunch(tag = "setRemoteChannel") {
val packetId = radioConfigUseCase.setRemoteChannel(destNum, channel)
@@ -311,7 +325,7 @@ open class RadioConfigViewModel(
}
fun setConfig(config: Config) {
- val destNum = destNode.value?.num ?: return
+ val destNum = destNum ?: destNode.value?.num ?: return
safeLaunch(tag = "setConfig") {
_radioConfigState.update { state ->
state.copy(
@@ -335,7 +349,7 @@ open class RadioConfigViewModel(
@Suppress("CyclomaticComplexMethod")
fun setModuleConfig(config: ModuleConfig) {
- val destNum = destNode.value?.num ?: return
+ val destNum = destNum ?: destNode.value?.num ?: return
safeLaunch(tag = "setModuleConfig") {
_radioConfigState.update { state ->
state.copy(
@@ -367,13 +381,13 @@ open class RadioConfigViewModel(
}
fun setRingtone(ringtone: String) {
- val destNum = destNode.value?.num ?: return
+ val destNum = destNum ?: destNode.value?.num ?: return
_radioConfigState.update { it.copy(ringtone = ringtone) }
safeLaunch(tag = "setRingtone") { radioConfigUseCase.setRingtone(destNum, ringtone) }
}
fun setCannedMessages(messages: String) {
- val destNum = destNode.value?.num ?: return
+ val destNum = destNum ?: destNode.value?.num ?: return
_radioConfigState.update { it.copy(cannedMessageMessages = messages) }
safeLaunch(tag = "setCannedMessages") { radioConfigUseCase.setCannedMessages(destNum, messages) }
}
@@ -420,12 +434,12 @@ open class RadioConfigViewModel(
}
fun setFixedPosition(position: Position) {
- val destNum = destNode.value?.num ?: return
+ val destNum = destNum ?: destNode.value?.num ?: return
safeLaunch(tag = "setFixedPosition") { radioConfigUseCase.setFixedPosition(destNum, position) }
}
fun removeFixedPosition() {
- val destNum = destNode.value?.num ?: return
+ val destNum = destNum ?: destNode.value?.num ?: return
safeLaunch(tag = "removeFixedPosition") { radioConfigUseCase.removeFixedPosition(destNum) }
}
@@ -456,7 +470,7 @@ open class RadioConfigViewModel(
}
fun installProfile(protobuf: DeviceProfile) {
- val destNum = destNode.value?.num ?: return
+ val destNum = destNum ?: destNode.value?.num ?: return
safeLaunch(tag = "installProfile") { installProfileUseCase(destNum, protobuf, destNode.value?.user) }
}
@@ -466,7 +480,7 @@ open class RadioConfigViewModel(
}
fun setResponseStateLoading(route: Enum<*>) {
- val destNum = destNumFlow.value ?: destNode.value?.num ?: return
+ val destNum = destNum ?: destNode.value?.num ?: return
_radioConfigState.update { it.copy(route = route.name, responseState = ResponseState.Loading()) }
@@ -613,7 +627,7 @@ open class RadioConfigViewModel(
}
private fun processPacketResponse(packet: MeshPacket) {
- val destNum = destNode.value?.num ?: return
+ val destNum = destNum ?: destNode.value?.num ?: return
val result = processRadioResponseUseCase(packet, destNum, requestIds.value) ?: return
val route = radioConfigState.value.route
diff --git a/feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModelTest.kt b/feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModelTest.kt
index 94d642f007..fc944d7d63 100644
--- a/feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModelTest.kt
+++ b/feature/settings/src/commonTest/kotlin/org/meshtastic/feature/settings/radio/RadioConfigViewModelTest.kt
@@ -402,19 +402,34 @@ class RadioConfigViewModelTest {
}
@Test
- fun `initDestNum updates value correctly including null`() = runTest {
- viewModel = createViewModel()
-
- // Initial setup should take the flow value, but let's just force update it
- viewModel.initDestNum(123)
- assertEquals(
- 123,
- viewModel.destNode.value?.num ?: 123,
- ) // the flow combine might need yielding, but we can just check it doesn't crash
-
- // The bug was that null was ignored. Here we test we can pass null
- // Since we can't easily read destNumFlow directly, we can just call it to ensure no crashes
- viewModel.initDestNum(null)
+ fun `destNum from SavedStateHandle resolves destNode`() = runTest {
+ val node = Node(num = 456, user = User(id = "!456"))
+ nodeRepository.setNodes(listOf(node))
+ viewModel =
+ RadioConfigViewModel(
+ savedStateHandle = SavedStateHandle(mapOf("destNum" to 456)),
+ radioConfigRepository = radioConfigRepository,
+ packetRepository = packetRepository,
+ serviceRepository = serviceRepository,
+ nodeRepository = nodeRepository,
+ locationRepository = locationRepository,
+ mapConsentPrefs = mapConsentPrefs,
+ analyticsPrefs = analyticsPrefs,
+ homoglyphEncodingPrefs = homoglyphEncodingPrefs,
+ toggleAnalyticsUseCase = toggleAnalyticsUseCase,
+ toggleHomoglyphEncodingUseCase = toggleHomoglyphEncodingUseCase,
+ importProfileUseCase = importProfileUseCase,
+ exportProfileUseCase = exportProfileUseCase,
+ exportSecurityConfigUseCase = exportSecurityConfigUseCase,
+ installProfileUseCase = installProfileUseCase,
+ radioConfigUseCase = radioConfigUseCase,
+ adminActionsUseCase = adminActionsUseCase,
+ processRadioResponseUseCase = processRadioResponseUseCase,
+ locationService = locationService,
+ fileService = fileService,
+ mqttManager = mqttManager,
+ )
+ assertEquals(456, viewModel.destNode.value?.num)
}
@Test
Served by rngit 1.5.2 - Generated in 0.1s